Correct contracts of Control#[get|set]Location() #790 #1454
+75
−42
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HeikoKlare
requested review from
lshanmug,
niraj-modi and
sravanlakkimsetti
as code owners
|
@akurtakov what do you think? |
The Control#getLocation() and Control#setLocation(...) operations currently claim to always work (i.e., yield reasonable values or actually change the location). This specification is incorrect when executing these methods in shells on a platform that uses the Wayland protocol. Since there is currently no fix to make the implementation fulfill the specification, this adds an according warning to the specification in order to avoid that clients rely on the specification to be fulfilled. Contributes to eclipse-platform#790
HeikoKlare
force-pushed
the
control-location-specification
branch
from
50ef6f8
to
6f544fc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
Control#getLocation()andControl#setLocation(...)operations currently claim to always work (i.e., yield reasonable values or actually change the location). This specification is incorrect when executing these methods in shells on a platform that uses the Wayland protocol.Currently, there does not seem to be a fix to make the implementation fulfill the specification. In case we claim that SWT can be used on Wayland, the specification thus has to be adapted to reflect that behavior. This change adds an according warning to the specification in order to avoid that clients rely on the specification to be fulfilled.
Currently, most UI elements requiring a separate shell/dialog are not placed as expected when using a platform with Wayland. In the Eclipse SDK, this includes the find/replace UIs (both the existing dialog and the new overlay), notification popups, quick type hierarchy, and other.
Having a warning in the specification of the broken methods is supposed to avoid further usage of those methods in problematic scenarios (to prevent avoidable regressions like #1447).
Why in Control instead of Shell?
We might also add the warning only to
Shell#[get|set]Location(), as it only applies to those methods executed on a shell. This But that would break the substitution principle as you might have a control at hand, not knowing that it's actually a shell that behaves differently.Contributes to #790